feat: add db writer, types, logging, util and test infrastructure#4
Conversation
Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
cendhu
left a comment
There was a problem hiding this comment.
I have done a first pass. Need to do a more in-depth review.
pkg/logging/logging.go
Outdated
| @@ -0,0 +1,165 @@ | |||
| /* | |||
There was a problem hiding this comment.
directly use flogging from fabric. https://pkg.go.dev/github.com/hyperledger/fabric/common/flogging
this local logging package can be removed.
pkg/types/types.go
Outdated
| TxID string | ||
| ValidationCode int32 | ||
| IsBlindWrite bool | ||
| ReadVersion *uint64 |
There was a problem hiding this comment.
I have a change of mind on this. I feel it would be more straight-forward and easy to understand read-only entries, read-write entries, blind-write entries rather than this optimization? what do you think? It will reflect the block structure as-is.
There was a problem hiding this comment.
@cendhu I agree this modular approach and I have updated the changed. Done
pkg/types/types.go
Outdated
| } | ||
|
|
||
| // TxNamespaceRecord represents a namespace within a transaction. | ||
| type TxNamespaceRecord struct { |
There was a problem hiding this comment.
This just covers one nsID and one nsVersion. The transaction might have touched multiple namespaces. EITHER the method name is wrong or the fields.
pkg/types/types.go
Outdated
| } | ||
|
|
||
| // ParsedBlockData contains writes, reads, and namespace records. | ||
| type ParsedBlockData struct { |
There was a problem hiding this comment.
We always place the caller first and then the callee. Move this method to the top of this file.
pkg/types/types.go
Outdated
|
|
||
| // ParsedBlockData contains writes, reads, and namespace records. | ||
| type ParsedBlockData struct { | ||
| Writes []WriteRecord |
There was a problem hiding this comment.
this write-records seem to have a lot of duplicates -- if a tx is writing 10 fields, for each field, we would be duplicating
Namespace string
BlockNum uint64
TxNum uint64
TxID string
pkg/constants/constants.go
Outdated
|
|
||
| // Namespace constants | ||
| const ( | ||
| MetaNamespaceID = "_meta" |
There was a problem hiding this comment.
import this from fabric-x-common
There was a problem hiding this comment.
@cendhu Done. I have removed the constant file, the plan is to import such constants from fabric-x-common or fabric-x-committer in future PRs, not define them locally.
pkg/constants/constants.go
Outdated
|
|
||
| // Transaction validation codes | ||
| const ( | ||
| StatusCommitted = 0 |
There was a problem hiding this comment.
import this from fabric-x-common
pkg/constants/constants.go
Outdated
| ) | ||
|
|
||
| // API default limits | ||
| const ( |
There was a problem hiding this comment.
what are these limits used for?
There was a problem hiding this comment.
@cendhu Done. The plan is to import such constants from fabric-x-common or fabric-x-committer in future PRs, not define them locally.
| } | ||
| }() | ||
|
|
||
| q := dbsqlc.New(tx) |
There was a problem hiding this comment.
can't you use batch in pgx to apply all changes in a single round-trip to the database? or is it taken care by the dbsqlc?
There was a problem hiding this comment.
@cendhu Done.. changed to batch processing
.stacked-pr-plan.md
Outdated
| @@ -0,0 +1,107 @@ | |||
| # Stacked PR Plan — fabric-x-block-explorer | |||
Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
…h pgx Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
…, tx_blind_writes Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
BlockInfo.Number already carries the block number. The top-level Number field on ProcessedBlock was never read and duplicated the same value set by the parser. Signed-off-by: Farooq Azam Wasim <Farooq.Azm.Wasim-CIC.Netherlands@ibm.com>
| func PrepareTestEnv(t *testing.T) *TestContainer { | ||
| t.Helper() | ||
|
|
||
| ctx := context.Background() |
There was a problem hiding this comment.
for test, we should always use t.Context()
| } | ||
|
|
||
| // cleanDatabase drops all tables for a clean test state. | ||
| func cleanDatabase(ctx context.Context, t *testing.T, pool *pgxpool.Pool) { |
There was a problem hiding this comment.
It is better to drop the whole database instead of tables.
| func TestDatabaseTestEnv(t *testing.T) { | ||
| env := NewDatabaseTestEnv(t) | ||
|
|
||
| ctx := context.Background() |
ParsedBlockData,ProcessedBlock,TxRecord,TxNamespaceRecord,ReadOnlyRecord,ReadWriteRecord,BlindWriteRecord,EndorsementRecord, andNamespacePolicyRecord; types are organized hierarchically to eliminate duplicate fieldsBlockWriterthat atomically persists a complete block including transactions, namespace entries, reads-only, read-writes, blind-writes, endorsements, and policies within a single DB transaction with rollback on error; all inserts use pgxSendBatchfor a single round-trip per tableBlockWriterincluding rollback on error, blind writes, multiple transactions, nil handling, and policy upsertsgolangci-lintconfiguration andlintMakefile targettest-no-db,test-requires-db,coverage, andcleantargets